From 8d5acdfc67530b0ee18cb07dced72a177372a999 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 25 Jun 2014 11:53:19 -0700 Subject: [PATCH] Don't recompile nested deps too frequently When compiling a package with a nested dependency, any modification to the outer package would trigger a recompilation of the inner package. This commit alters the fingerprint() method to take a PackageId to query about the location of a package and only lookup the files relevant to that package. The dependency structure of a PathSource is now everything rooted at the original Cargo.toml minus all subdirectories which contain a Cargo.toml --- src/cargo/core/package.rs | 2 +- src/cargo/core/source.rs | 9 +++-- src/cargo/sources/git/source.rs | 2 +- src/cargo/sources/path.rs | 45 ++++++++++++++++------- tests/test_cargo_compile_path_deps.rs | 53 +++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 18 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 32284f82f..b9cc31921 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -128,7 +128,7 @@ impl Package { let sources = sources.iter().map(|source_id| { source_id.load(config) }).collect::>(); - SourceSet::new(sources).fingerprint() + SourceSet::new(sources).fingerprint(self) } } diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index 3214a8eb0..6e7192a35 100644 --- a/src/cargo/core/source.rs +++ b/src/cargo/core/source.rs @@ -39,7 +39,10 @@ pub trait Source { /// This fingerprint is used to determine the "fresheness" of the source /// later on. It must be guaranteed that the fingerprint of a source is /// constant if and only if the output product will remain constant. - fn fingerprint(&self) -> CargoResult; + /// + /// The `pkg` argument is the package which this fingerprint should only be + /// interested in for when this source may contain multiple packages. + fn fingerprint(&self, pkg: &Package) -> CargoResult; } #[deriving(Show, Clone, PartialEq, Eq, PartialOrd, Ord)] @@ -203,10 +206,10 @@ impl Source for SourceSet { Ok(ret) } - fn fingerprint(&self) -> CargoResult { + fn fingerprint(&self, id: &Package) -> CargoResult { let mut ret = String::new(); for source in self.sources.iter() { - ret.push_str(try!(source.fingerprint()).as_slice()); + ret.push_str(try!(source.fingerprint(id)).as_slice()); } return Ok(ret); } diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 1868023c0..97462af20 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -133,7 +133,7 @@ impl<'a, 'b> Source for GitSource<'a, 'b> { self.path_source.get(ids) } - fn fingerprint(&self) -> CargoResult { + fn fingerprint(&self, _pkg: &Package) -> CargoResult { let db = self.remote.db_at(&self.db_path); db.rev_for(self.reference.as_slice()) } diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 225b35c02..26baccd57 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -48,6 +48,14 @@ impl PathSource { None => Err(internal("no package found in source")) } } + + fn read_packages(&self) -> CargoResult> { + if self.updated { + Ok(self.packages.clone()) + } else { + ops::read_packages(&self.path, &self.id) + } + } } impl Show for PathSource { @@ -59,9 +67,9 @@ impl Show for PathSource { impl Source for PathSource { fn update(&mut self) -> CargoResult<()> { if !self.updated { - let pkgs = try!(ops::read_packages(&self.path, &self.id)); - self.packages.push_all_move(pkgs); - self.updated = true; + let packages = try!(self.read_packages()); + self.packages.push_all_move(packages); + self.updated = true; } Ok(()) @@ -87,17 +95,28 @@ impl Source for PathSource { .collect()) } - fn fingerprint(&self) -> CargoResult { - let mut max = None; - let target_dir = self.path.join("target"); - for child in try!(fs::walk_dir(&self.path)) { - if target_dir.is_ancestor_of(&child) { continue } - let stat = try!(fs::stat(&child)); - max = cmp::max(max, Some(stat.modified)); + fn fingerprint(&self, pkg: &Package) -> CargoResult { + let packages = try!(self.read_packages()); + let mut max = 0; + for pkg in packages.iter().filter(|p| *p == pkg) { + let loc = pkg.get_manifest_path().dir_path(); + max = cmp::max(max, try!(walk(&loc, true))); } - match max { - None => Ok(String::new()), - Some(time) => Ok(time.to_str()), + return Ok(max.to_str()); + + fn walk(path: &Path, is_root: bool) -> CargoResult { + if !path.is_dir() { + return Ok(try!(fs::stat(path)).modified) + } + // Don't recurse into any sub-packages that we have + if !is_root && path.join("Cargo.toml").exists() { return Ok(0) } + + let mut max = 0; + for dir in try!(fs::readdir(path)).iter() { + if is_root && dir.filename_str() == Some("target") { continue } + max = cmp::max(max, try!(walk(dir, false))); + } + return Ok(max) } } } diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs index f3063d678..635808eb1 100644 --- a/tests/test_cargo_compile_path_deps.rs +++ b/tests/test_cargo_compile_path_deps.rs @@ -303,3 +303,56 @@ test!(no_rebuild_two_deps { FRESH, bar.display(), FRESH, p.root().display()))); }) + +test!(nested_deps_recompile { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [dependencies.bar] + + version = "0.5.0" + path = "src/bar" + + [[bin]] + + name = "foo" + "#) + .file("src/foo.rs", + main_file(r#""{}", bar::gimme()"#, ["bar"]).as_slice()) + .file("src/bar/Cargo.toml", r#" + [project] + + name = "bar" + version = "0.5.0" + authors = ["wycats@example.com"] + + [[lib]] + + name = "bar" + "#) + .file("src/bar/src/bar.rs", "pub fn gimme() {}"); + let bar = p.root(); + + assert_that(p.cargo_process("cargo-build"), + execs().with_stdout(format!("{} bar v0.5.0 (file:{})\n\ + {} foo v0.5.0 (file:{})\n", + COMPILING, bar.display(), + COMPILING, p.root().display()))); + // See comments for the above `sleep` + timer::sleep(1000); + File::create(&p.root().join("src/foo.rs")).write_str(r#" + fn main() {} + "#).assert(); + + // This shouldn't recompile `bar` + assert_that(p.process("cargo-build"), + execs().with_stdout(format!("{} bar v0.5.0 (file:{})\n\ + {} foo v0.5.0 (file:{})\n", + FRESH, bar.display(), + COMPILING, p.root().display()))); +}) -- 2.30.2